fix: treat empty on-disk JSON state files as empty state by mwbrooks · Pull Request #616 · slackapi/slack-cli

mwbrooks · 2026-07-15T18:26:36Z

Changelog

Fixed a recurring unable_to_parse_json error caused by empty JSON files (credentials, config, or app state) left behind by an interrupted write - the CLI now treats them as empty state and recovers automatically.

Summary

This pull request helps to address unable_to_parse_json internal errors that some users are experiencing.

Problem

afero.WriteFile opens with O_TRUNC before writing. If the process is interrupted (SIGKILL, host crash, oom, user cancel) after the truncate but before the write completes, the JSON state file is left at zero bytes. Every subsequent CLI invocation then reads the file, calls json.Unmarshal(nil, …) on the empty contents, and raises ErrUnableToParseJSON. The file never self-heals: the reader errors out before the caller reaches the code path that would rewrite it.

Affected state files:

~/.slack/credentials.json
~/.slack/config.json
Project .slack/config.json
Project .slack/apps.json
Project .slack/apps.dev.json

What this PR changes

Adds a single helper goutils.IsEmptyJSON([]byte) bool and updates the five readers so that an empty (or whitespace-only) file returns the same zero-value state as the "file does not exist" branch, rather than surfacing a parse error:

internal/auth/auth.go — auths() returns an empty AuthByTeamID
internal/config/system.go — UserConfig returns a config with Surveys initialised
internal/config/project.go — ReadProjectConfigFile returns a config with Surveys initialised
internal/app/app_client.go — readDeployedApps and readLocalApps return empty maps and re-serialise a valid {} so the file self-heals on the next write cycle

Genuinely malformed JSON still returns ErrUnableToParseJSON. Existing broken-JSON tests are untouched.

Test plan

Test_IsEmptyJSON covers empty, whitespace-only, {}, and populated inputs
Test_Client_auths empty-file subtest returns no auths, no error
Test_SystemConfig_UserConfig empty-file subtest returns zero-valued config, no error
Test_ReadProjectConfigFile empty-file subtest returns zero-valued config, no error
AppClient.readDeployedApps empty-file subtest returns empty apps and writes {} back to disk
AppClient.readLocalApps empty-file subtest returns empty local apps and writes {} back to disk
go test ./internal/... ./cmd/... passes
make lint passes with zero issues

Follow-ups (not in this PR)

Atomic writes — replace afero.WriteFile with a temp-file + Rename pattern across the five state writers so the durable fix is that these files can never reach a 0-byte state in the first place.
Centralise the read-JSON-state pattern — the "stat → read → check empty → unmarshal → wrap error" sequence is now duplicated across five call sites. Move it into goutils (or a new internal/statefile package) so future state files pick up the guard for free.
internal/api/activity.go — ErrHTTPResponseInvalid on the activity poll loop shows the same "thousands of events off one broken response" shape; worth a matching guard.
internal/update/sdk.go — retry with backoff on persistently malformed SDK hook response so a broken SDK doesn't dominate the error telemetry either.

… error Truncated writes to the CLI's JSON state files (credentials.json, ~/.slack/config.json, project .slack/config.json, .slack/apps.json, .slack/apps.dev.json) can leave a file at zero bytes on disk when a prior process is interrupted between afero.WriteFile's O_TRUNC and the actual write. Once that happens, every subsequent CLI invocation reads the same 0-byte file, hits "unexpected end of JSON input" in the read helper, and raises ErrUnableToParseJSON. The June 2026 monthly report showed 1,317,610 unable_to_parse_json events from just 38 users (a hot loop concentrated in a small cohort running `platform run` and `api` in tight automation loops), which matches this failure mode: a file that never repairs itself keeps re-triggering on every invocation. Add a shared goutils.IsEmptyJSON helper and use it in the five hot readers (auth credentials, system config, project config, deployed apps, local dev apps). When the file exists but is empty or whitespace-only, return the same zero-value state the "file does not exist" branch already returns — the app_client readers also re-serialise a valid `{}` so the file self-heals on the next successful write. Genuinely malformed JSON still returns ErrUnableToParseJSON as before. Test coverage: added unit tests covering zero-byte and whitespace-only inputs for each of the five readers, plus a unit test for the IsEmptyJSON helper itself. Existing "broken JSON" tests are unchanged.

codecov · 2026-07-15T18:32:46Z

Codecov Report

❌ Patch coverage is 81.81818% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.79%. Comparing base (e27054f) to head (6d374f8).

Files with missing lines	Patch %	Lines
internal/app/app_client.go	66.66%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #616      +/-   ##
==========================================
+ Coverage   71.73%   71.79%   +0.06%     
==========================================
  Files         227      227              
  Lines       19261    19283      +22     
==========================================
+ Hits        13817    13845      +28     
+ Misses       4228     4224       -4     
+ Partials     1216     1214       -2

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

mwbrooks

Comments for the kind reviewers

mwbrooks · 2026-07-16T05:46:55Z

+	// every subsequent CLI invocation reads the same 0-byte file and logs
+	// ErrUnableToParseJSON in a hot loop.
+	if goutils.IsEmptyJSON(configFileBytes) {
+		config.Surveys = map[string]SurveyConfig{}


note: I thought there might be an initialize function, but this same pattern is used elsewhere.

zimeg

@mwbrooks LGTM! I think this is alright to merge with plans to follow up with "atomic" writes. I emphasize this because guarding against these empty error cases isn't as ideal as preventing such. The pattern you suggest is fascinating to me 🧠 💡

I'm thinking we should merge this for next release after finding testing performs as expected!

zimeg · 2026-07-17T19:55:01Z

+	// Treat an empty (or whitespace-only) file as "no apps saved yet" rather
+	// than a parse error. This state occurs when a prior write was truncated
+	// but not completed (e.g. a process was interrupted between afero.WriteFile's
+	// O_TRUNC and the actual write). Without this guard, every subsequent CLI
+	// invocation logs ErrUnableToParseJSON on the same file.


🔭 thought: The follow up "atomic" writes is a clever and good workaround to have! I'm worried this patch might cause existing project apps to seem missing instead of a JSON error appearing, but the root cause is unrelated to this PR.

zimeg · 2026-07-17T20:03:10Z

+	// Treat an empty (or whitespace-only) credentials file as "no credentials
+	// stored" rather than a parse error. This state occurs when a prior write
+	// was truncated but not completed (e.g. a process was interrupted between
+	// afero.WriteFile's O_TRUNC and the actual write). Without this guard,
+	// every subsequent CLI invocation reads the same 0-byte file and logs
+	// ErrUnableToParseJSON in a hot loop.
+	if goutils.IsEmptyJSON(raw) {
+		return types.AuthByTeamID{}, nil
+	}
+
 	err = json.Unmarshal(raw, &auths)


👾 thought: Perhaps covered in the "atomic" reads and writes once more, but I find these functions pair well together and am curious if our own JSON package might handle blank files instead of addressing this at each callsite?

zimeg · 2026-07-17T20:04:27Z

+func IsEmptyJSON(data []byte) bool {
+	return len(bytes.TrimSpace(data)) == 0
+}


🪬 quibble: Calling this "empty JSON" to me is confusing because I'd expect:

{}

And instead might prefer to call this IsBlankFile or something similar but of course the description above is amazing at making this clear.

srtaalej

tested and it looks great!

mwbrooks added 2 commits July 15, 2026 13:51

Merge branch 'main' into mwbrooks-fix-unable-to-parse-json

4f63517

chore: drop internal telemetry reference from comment

77804be

mwbrooks self-assigned this Jul 16, 2026

mwbrooks added bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented changelog Use on updates to be included in the release notes semver:patch Use on pull requests to describe the release version increment labels Jul 16, 2026

mwbrooks commented Jul 16, 2026

View reviewed changes

mwbrooks marked this pull request as ready for review July 16, 2026 23:10

mwbrooks requested a review from a team as a code owner July 16, 2026 23:10

zimeg added this to the Next Release milestone Jul 17, 2026

Merge branch 'main' into mwbrooks-fix-unable-to-parse-json

6d374f8

zimeg approved these changes Jul 17, 2026

View reviewed changes

srtaalej approved these changes Jul 17, 2026

View reviewed changes

zimeg merged commit c68950e into main Jul 17, 2026
10 checks passed

zimeg deleted the mwbrooks-fix-unable-to-parse-json branch July 17, 2026 20:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: treat empty on-disk JSON state files as empty state#616

fix: treat empty on-disk JSON state files as empty state#616
zimeg merged 4 commits into
mainfrom
mwbrooks-fix-unable-to-parse-json

mwbrooks commented Jul 15, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Jul 15, 2026 •

edited

Loading

Uh oh!

mwbrooks left a comment

Uh oh!

mwbrooks Jul 16, 2026

Uh oh!

zimeg left a comment

Uh oh!

zimeg Jul 17, 2026

Uh oh!

zimeg Jul 17, 2026

Uh oh!

zimeg Jul 17, 2026

Uh oh!

srtaalej left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

mwbrooks commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog

Summary

Problem

What this PR changes

Test plan

Follow-ups (not in this PR)

Uh oh!

codecov Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mwbrooks left a comment

Choose a reason for hiding this comment

Uh oh!

mwbrooks Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

zimeg left a comment

Choose a reason for hiding this comment

Uh oh!

zimeg Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

zimeg Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

zimeg Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

srtaalej left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mwbrooks commented Jul 15, 2026 •

edited

Loading

codecov Bot commented Jul 15, 2026 •

edited

Loading